deps: bump up dd_trace_cpp to v1.0.0 #39409
Conversation
repokitteh-read-only
bot
added
the
deps
|
CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to |
agrawroh
force-pushed
the
deps-dd_trace_cpp
branch
from
43e86ac to
72ba8bd
Compare
Signed-off-by: Rohit Agrawal <[email protected]>
agrawroh
force-pushed
the
deps-dd_trace_cpp
branch
from
72ba8bd to
aa2447b
Compare
|
/assign @phlax |
|
/lgtm deps |
repokitteh-read-only
bot
removed
the
deps
There was a problem hiding this comment.
LGTM, thanks!
I've left a few minor suggestions.
Are you also expected to take ownership of the extension (
Line 67 in 8b9c389
If not, this should probably be approved by the current code-owners
| /** | ||
| * Implementation of the required config() method from datadog::tracing::HTTPClient | ||
| */ | ||
| std::string config() const override; |
There was a problem hiding this comment.
nit: consider returning a const ref (or string_view) to avoid unneeded copies.
There was a problem hiding this comment.
Hmmm, it doesn't match the base class's virtual function signature which returns by value.
./source/extensions/tracers/datadog/agent_http_client.h:89:22: error: virtual function 'config' has a different return type ('const std::string &' (aka 'const basic_string<char> &')) than the function it overrides (which has return type 'std::string' (aka 'basic_string<char>'))
89 | const std::string& config() const override;
| ~~~~~~~~~~~~ ^
bazel-out/darwin_arm64-opt/bin/external/com_github_datadog_dd_trace_cpp/_virtual_includes/dd_trace_cpp/datadog/http_client.h:64:23: note: overridden virtual function is here
64 | virtual std::string config() const = 0;
| ~~~~~~~~~~~ ^
|
cc @dmehala @mattklein123 Could you please take a look as the extension owners? |
Signed-off-by: Rohit Agrawal <[email protected]>
repokitteh-read-only
bot
added
the
deps
Signed-off-by: Rohit Agrawal <[email protected]>
|
From what I recall, the last time I internally updated to version 1.0.0, Envoy began to crash (though this was some time ago and I was about to go on holiday). I will test it internally and keep you updated. |
It might be worth adding a few integration tests regardless. I can try to work on it. |
|
@dmehala Did you get a chance to try it on your side? |
|
@agrawroh yes, we’re currently dogfooding on our end. Would it be reasonable to wait until Monday? That would give us sufficient time to test everything thoroughly. |
|
@dmehala have you tried it on your end? Is this PR ready to be merged? Thanks |
|
Hey @tyxia, we've got it running internally, but unfortunately, we hit several crashes because of the telemetry module in |
|
/wait Then I just added a wait tag for now? |
Description
This PR bumps up the version of
dd_trace_cppto v1.0.0.The
v1.0.0release ofdd-trace-cpprepresents a stable API that should provide better long-term API compatibility compared to pre-1.0 versions. Upgrading now reduces technical debt and ensures we're using the most current implementation with all fixes and improvements.API Changes:
The v1.0.0 release includes some breaking API changes that required code modifications. These changes improve the library's usability and alignment with DataDog's other language implementations. Key changes include revised initialization patterns and updated configuration interfaces.
Reference:
For complete details on the v1.0.0 release: https://github.com/DataDog/dd-trace-cpp/releases/tag/v1.0.0
Commit Message: deps: bump up
dd_trace_cppto v1.0.0Additional Description: Bump up the version of
dd_trace_cppto v1.0.0Risk Level: Low
Testing: CI
Docs Changes: N/A
Release Notes: N/A